Add return-type to public functions, mostly tests part 4 #7645
Conversation
No change in the effective code. A batch of ~50 files. Modified files pass ruff check --select=ANN201 Partially implements quantumlib#4393
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7645 +/- ##
==========================================
- Coverage 99.37% 99.37% -0.01%
==========================================
Files 1078 1078
Lines 96166 96212 +46
==========================================
+ Hits 95563 95608 +45
- Misses 603 604 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| self._memo: dict[Any, dict] = {} | ||
|
|
||
| def default(self, o): | ||
| def default(self, o) -> Any: |
There was a problem hiding this comment.
It seems that maybe we could be more specific. Perhaps something like
def default(self, o) -> bool | list[Any] | dict[str, Any]:
...might work. Maybe one could be even more specific?
Note 1: It is useful to know that if the return type isn't a bool then it's something one can iterate over, and that if it's not a list, then we can iterate over its keys and rely on them being strings.
Note 2: A better opportunity to engineer neat narrow type annotations for these functions is unlikely to materialize in future.
There was a problem hiding this comment.
Good point, done in 96e4cbe.
| def measurement_key_obj(val: Any, default: Any = RaiseTypeErrorIfNotProvided): | ||
| def measurement_key_obj( | ||
| val: Any, default: TDefault = RaiseTypeErrorIfNotProvided | ||
| ) -> cirq.MeasurementKey | TDefault: |
There was a problem hiding this comment.
I think this function never returns the type TDefault of default, so it would be nice not to have to deal with the alternative at the callsites. It looks like type inference failure in
if default is not RaiseTypeErrorIfNotProvided:
return defaultIs there a way to explain to mypy better what's actually going on?
There was a problem hiding this comment.
Yes, we can use the @overload decorator to distinguish calls without the default when the return type is certain. When default is provided, the output can be still either of MeasurementKey | TDefault. Thanks for the suggestion, done in 0709e48.
|
|
||
|
|
||
| def with_measurement_key_mapping(val: Any, key_map: Mapping[str, str]): | ||
| def with_measurement_key_mapping(val: Any, key_map: Mapping[str, str]) -> Any: |
There was a problem hiding this comment.
Can we do better than Any here?
There was a problem hiding this comment.
Good point. We can use a generic type T for the val type and expect the same type to be returned. Done in 0709e48.
Specify types that can be actually returned rather than Any.
Annotate with `@overload` for the return type is known when the `default` is not provided.
3 participants
Add return-type to public functions, mostly tests part 4
No change in the effective code. A batch of ~50 files.
Modified files pass ruff check --select=ANN201
Partially implements #4393